Skip to content

Block attempts to play unsupported WebM streams with DRM#2020

Merged
CastagnaIT merged 1 commit intoxbmc:Piersfrom
CastagnaIT:webm_drm_unsupported
Apr 20, 2026
Merged

Block attempts to play unsupported WebM streams with DRM#2020
CastagnaIT merged 1 commit intoxbmc:Piersfrom
CastagnaIT:webm_drm_unsupported

Conversation

@CastagnaIT
Copy link
Copy Markdown
Collaborator

@CastagnaIT CastagnaIT commented Apr 20, 2026

Description

"fix" issue #2018

this will block attempts to play unsupported videos

Motivation and context

Block attempts to play unsupported WebM streams with DRM
this is currently not implemented on webm demuxer and also its needed to verify possible changes on CDM side

this is a know unsupported feature (already on wiki)
there is no plan from my part in to implementing it in the near future

for reference on ISA Samples there is a sample stream that allow to test this use case:
the stream: Dash->VOD DRM->Google tears [default_KID on DRM CP]

How has this been tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@CastagnaIT CastagnaIT added Type: Fix non-breaking change which fixes an issue v22 Piers labels Apr 20, 2026
@CastagnaIT CastagnaIT linked an issue Apr 20, 2026 that may be closed by this pull request
[Bug] [VP9] decoding video segments issue #2018
Closed
github-actions[bot]
github-actions Bot reviewed Apr 20, 2026
View reviewed changes
Comment thread src/main.cpp Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

Must Fix (Medium)

Check placement after video stream re-linking (src/main.cpp:287–293)

The DRM guard fires after the included-stream re-linking loop (lines 271–285). When the failing stream is a WebM video stream, that loop has already unlinked audio streams from the previous video reader and re-linked them to this one before return false is reached. Those audio streams are then dangling — linked to an unenabled, un-SetStreamId'd reader — until a fallback video stream opens and re-links them again.

The fix is to move the new block to immediately after the PrepareStream check (line 264), before SetStreamId and before the re-linking loop. Both GetType() == WebM and GetSessionId().empty() require PrepareStream to have run (the reader and DRM session are created there), so the earliest valid placement is right after line 264.

@kodiai
Copy link
Copy Markdown

kodiai Bot commented Apr 20, 2026 •
edited
Loading

Kodiai Review Summary

What Changed

Adds validation to block playback attempts for WebM streams with DRM, which are currently unsupported by the WebM demuxer.

Reviewed: core logic

Strengths

  • ✅ Clear error logging helps users understand why playback failed (src/main.cpp:291)
  • ✅ Check is properly placed before EnableStream to prevent initialization of unsupported configurations (src/main.cpp:287-295)
  • ✅ TODO comment documents future implementation requirements for DRM and CDM changes (src/main.cpp:290)

Observations

Impact

[MINOR] src/main.cpp (287-288): Redundant GetReader() call
The condition calls stream->GetReader() twice - once for the null check and again to get the type. This is inefficient and could be avoided by storing the result in a local variable.

Suggestions

  • Optional: Consider placing this check earlier in the function (before line 266) to avoid unnecessary SetStreamId call for streams that will be blocked anyway

Verdict

🟡 Ready to merge with minor items -- Optional cleanup suggestions below (no blockers)

Review Details

  • Files reviewed: 1

  • Findings: 0 critical, 0 major, 0 medium, 2 minor (includes 1 from summary observations)

  • Lines changed: +8 -0

  • Profile: strict (auto, lines changed: 8)

  • Contributor experience: profile-backed (using linked contributor profile guidance)

  • Review completed: 2026-04-20T16:20:29.747Z

  • Total wall-clock: 4m 27s

  • Phase timings:

    • queue wait: 0ms
    • workspace preparation: 1.3s
    • retrieval/context assembly: 10.0s
    • executor handoff: 4.4s
    • remote runtime: 4m 8s
    • publication: 1.0s

  • Tokens: 100 in / 8,216 out | 0.2670

  • Keyword parsing: No keywords detected

kodiai[bot]
kodiai Bot reviewed Apr 20, 2026
View reviewed changes
Comment thread src/main.cpp Outdated
@CastagnaIT CastagnaIT force-pushed the webm_drm_unsupported branch from 15327c2 to 7e50a1d Compare April 20, 2026 17:15
@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

CastagnaIT commented Apr 20, 2026

@kodiai please review

kodiai[bot]
kodiai Bot approved these changes Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown

@kodiai kodiai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kodiai response

Decision: APPROVE
Issues: none

Evidence:

  • Review prompt covered 1 changed file.
  • Repo inspection tools were used to verify the changed code.

@CastagnaIT CastagnaIT merged commit f6deb2e into xbmc:Piers Apr 20, 2026
11 checks passed
@CastagnaIT CastagnaIT deleted the webm_drm_unsupported branch April 20, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@kodiai kodiai[bot] kodiai[bot] approved these changes

Assignees

No one assigned

Labels

Type: Fix non-breaking change which fixes an issue v22 Piers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] [VP9] decoding video segments issue

1 participant

@CastagnaIT
⚡